`build/pkgs/setuptools_scm`: Update to 8.0.4, add fixes for version 8 #36400

mkoeppe · 2023-10-04T17:04:41Z

Version 8 needs some new workarounds when packages that do not use setuptools_scm are installed with --no-build-isolation in an environment with setuptools_scm. This is relevant in particular for conda, see #35593.

setuptools_scm inserts itself into the build process of unsuspecting packages via entry points declared by setuptools. One gets a warning:

[sage_conf-10.2.beta5] WARNING setuptools_scm._integration.setuptools pyproject.toml does not contain a tool.setuptools_scm section (https://github.com/sagemath/sage/actions/runs/6409373032/job/17400573034?pr=36400#step:8:3978)

These warnings show up at build time, but also in runtime uses of Cython, where it causes doctest failures.

Here we do the update (for no specific reason other than to be up to date) and implement the workarounds.

📝 Checklist

The title is concise, informative, and self-explanatory.
The description explains in detail what this PR is about.
I have linked a relevant issue or discussion.
I have created tests covering the changes.
I have updated the documentation accordingly.

⌛ Dependencies

kiwifb · 2023-10-04T21:31:52Z

I would consider those warnings an upstream bug. The way you word it ("unsuspecting"), I almost feel like having a conversation on consent in software packaging.

mkoeppe · 2023-10-04T21:39:20Z

Offering these setuptools entry points that modify other packages' build behavior has always seemed like a severe & puzzling misdesign to me, but I may be missing something. In what is now the default in Python dev, using build isolation, it's not a problem any more -- each package just declares its PEP517/518 build environment, and if it does not like to use setuptools_scm, it just doesn't use it. But in flat installations as in Linux distributions, the problem persists.

kiwifb · 2023-10-04T21:47:21Z

I was a bit sarcastic. And I have not looked for that issue in my logs so I cannot tell if it is a big problem. But if if the configuration bits is missing, may be it should be a signal that your tool is not required and that's the end of that. There is plenty of software available on a computer and they do not all jump and down saying "use me! use me!". What's wrong with some people.

github-actions · 2023-10-05T02:15:36Z

Documentation preview for this PR (built with commit 114a610; changes) is ready! 🎉

tobiasdiez

I don't quite understand the motivation / problem in the context of conda. We don' use setuptools_scm so why would we deliberately install it in the conda environment, especially if it causes problems? I'm also not aware that I've seen this error before, so it feels more like an issue with the implementation in #35593.

mkoeppe · 2023-10-05T03:03:15Z

I don't quite understand the motivation / problem in the context of conda.

What's particular about conda (in contrast to normal modern python dev) is that one commonly uses --disable-build-isolation -- so that the packages from the conda environment are used.

We don' use setuptools_scm so why would we deliberately install it in the conda environment, especially if it causes problems?

We wouldn't ideally, but users may (for example and in particular when people switch between different ways of using the same conda environment). It is important that just installing this package does not break stuff.

dimpase · 2023-10-16T11:28:27Z

Can't we get rid of the special treatment of Conda (in particular --no-build-isolation) by means of --enable-system-site-packages ?

dimpase

I'm approving this for the time being, but in general this looks like a less than ideal design.

tobiasdiez · 2023-10-16T12:49:01Z

no-build-isolation when installing sagelib is need since you want to use the build dependencies that are already installed by conda in the environment, and not get new ones via pip, see eg conda/conda-build#4251 (comment)

But maybe it is sufficient to voice an warning (during configure) that some doctest may break when setuptools_scm is present in the conda environment. If one has more packages installed in the conda env than what we specify, it is logical that this may cause issues. And the setup_scm failure doesn't seem to be that bad.

dimpase · 2023-10-16T13:02:05Z

no-build-isolation when installing sagelib is need since you want to use the build dependencies that are already installed by conda in the environment, and not get new ones via pip, see eg conda/conda-build#4251 (comment)

How is this not applicable to the --enable-system-site-packages case? Are you saying there is no other way to limit pip in its quest to install the latest stuff no matter what? Isn't pip here coming from Conda, yet they cannot rein it in? What an OS (e.g. a Linux distro) normally does in such a case, is limiting the use of pip to only do --user case - for non-root installs.

tobiasdiez · 2023-10-16T15:36:33Z

no-build-isolation when installing sagelib is need since you want to use the build dependencies that are already installed by conda in the environment, and not get new ones via pip, see eg conda/conda-build#4251 (comment)

How is this not applicable to the --enable-system-site-packages case?

I guess it is. And if you install sagelib with make it also uses the switch:

sage/build/bin/sage-dist-helpers

Lines 291 to 297 in b7c1c8f

    
           sdh_pip_editable_install() { 
        
               echo "Installing $PKG_NAME (editable mode)" 
        
               # Until https://github.com/sagemath/sage/issues/34209 switches us to PEP 660 editable wheels 
        
               export SETUPTOOLS_ENABLE_FEATURES=legacy-editable 
        
               python3 -m pip install --verbose --no-deps --no-index --no-build-isolation --isolated --editable "$@" || \ 
        
                   sdh_die "Error installing $PKG_NAME" 
        
           }

Are you saying there is no other way to limit pip in its quest to install the latest stuff no matter what?

As far as I know pip doesn't have a switch to "use build isolation, but take build system dependencies from the global environment if they meet the requirements".

mkoeppe · 2023-10-17T00:54:39Z

As far as I know pip doesn't have a switch to "use build isolation, but take build system dependencies from the global environment if they meet the requirements".

That's right. It's all or nothing. The isolated build environment is created from scratch; it never takes anything from the site packages (user or system). The only way to control it is via pip's general options regarding index servers. This is what we do when building packages in the Sage distribution; we disallow the use of the PyPI index server (--no-index) and only give it our fixed set of wheels in venv/var/libs/sage/wheels via --find-links.

kiwifb · 2023-10-17T01:08:30Z

Needs rebasing. The CI failures seem unrelated, I'll join Dima in saying we can move forwards with this.

mkoeppe · 2023-10-17T01:10:18Z

it merges cleanly; not a good idea to merge/rebase when it's already in positive review

kiwifb · 2023-10-17T01:14:12Z

Cool, I was interpreting github's message a little bit more severely.

…ixes for version 8    Version 8 needs some new workarounds when packages that do not use `setuptools_scm` are installed with `--no-build-isolation` in an environment with `setuptools_scm`. This is relevant in particular for conda, see sagemath#35593. `setuptools_scm` inserts itself into the build process of unsuspecting packages via entry points declared by setuptools. One gets a warning: - `[sage_conf-10.2.beta5] WARNING setuptools_scm._integration.setuptools pyproject.toml does not contain a tool.setuptools_scm section` (https://github.com/sagemath/sage/actions/r uns/6409373032/job/17400573034?pr=36400#step:8:3978) These warnings show up at build time, but also in runtime uses of Cython, where it causes doctest failures. Here we do the update (for no specific reason other than to be up to date) and implement the workarounds.   ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36400 Reported by: Matthias Köppe Reviewer(s): Dima Pasechnik, Tobias Diez

Since 10.2.beta8, likely as a side effect of the setuptools_scm update (sagemath#36400), some of our modularized sdists contain way too many files. https://pypi.org/project/sagemath-bliss/10.2b8/#files We fix this by updating the MANIFEST.in files. - Cherry-picked from sagemath#35095.    ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36520 Reported by: Matthias Köppe Reviewer(s): François Bissey

tobiasdiez · 2023-10-26T17:16:45Z

Instead of suppressing the warning, it now shows up in the doctests leading to build errors (also can be seen here on this PR: https://github.com/sagemath/sage/actions/runs/6412079178/job/17409624223). Could you please fix it in a follow-up?

kiwifb · 2023-10-26T17:31:40Z

I honestly think this is an upstream bug. "WARNING" is not the right log level for telling you you are not using an optional feature. "INFO" would be more appropriate.

mkoeppe · 2023-10-26T17:54:20Z

I honestly think this is an upstream bug

I agree. Nevertheless we should work around it in our runtime use of Cython.

mkoeppe · 2023-10-26T18:21:25Z

the warning [...] shows up in the doctests

Indeed this happens when the doctester is invoked from the src directory, or probably any directory that contains a setup.cfg or similar.

Since 10.2.beta8, likely as a side effect of the setuptools_scm update (sagemath#36400), some of our modularized sdists contain way too many files. https://pypi.org/project/sagemath-bliss/10.2b8/#files We fix this by updating the MANIFEST.in files. - Cherry-picked from sagemath#35095.    ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36520 Reported by: Matthias Köppe Reviewer(s): François Bissey

…etuptools_scm    The part of sagemath#36400 that was intended to fix the runtime use of cython did not actually fix it because `finalize_distribution` is called already by `Distribution.__init__`. This shows up when invoked from a directory with `pyproject.toml` or a similar config file. As the Build & Test workflow invokes the doctester from `SAGE_ROOT/src`, these errors show up in all tests. Marking it as a blocker so it is [applied automatically in CI workflows](https://github.com/sagemath/sage/wiki/Sage-10.2-Release- Tour#open-blocker-prs-are-applied-automatically-in-ci-workflows)  Fixes sagemath#36400 (comment)  ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [ ] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36552 Reported by: Matthias Köppe Reviewer(s): Tobias Diez

Since 10.2.beta8, likely as a side effect of the setuptools_scm update (sagemath#36400), some of our modularized sdists contain way too many files. https://pypi.org/project/sagemath-bliss/10.2b8/#files We fix this by updating the MANIFEST.in files. - Cherry-picked from sagemath#35095.    ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36520 Reported by: Matthias Köppe Reviewer(s): François Bissey

…etuptools_scm    The part of sagemath#36400 that was intended to fix the runtime use of cython did not actually fix it because `finalize_distribution` is called already by `Distribution.__init__`. This shows up when invoked from a directory with `pyproject.toml` or a similar config file. As the Build & Test workflow invokes the doctester from `SAGE_ROOT/src`, these errors show up in all tests. Marking it as a blocker so it is [applied automatically in CI workflows](https://github.com/sagemath/sage/wiki/Sage-10.2-Release- Tour#open-blocker-prs-are-applied-automatically-in-ci-workflows)  Fixes sagemath#36400 (comment)  ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [ ] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36552 Reported by: Matthias Köppe Reviewer(s): Tobias Diez

Since 10.2.beta8, likely as a side effect of the setuptools_scm update (sagemath#36400), some of our modularized sdists contain way too many files. https://pypi.org/project/sagemath-bliss/10.2b8/#files We fix this by updating the MANIFEST.in files. - Cherry-picked from sagemath#35095.    ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36520 Reported by: Matthias Köppe Reviewer(s): François Bissey

…etuptools_scm    The part of sagemath#36400 that was intended to fix the runtime use of cython did not actually fix it because `finalize_distribution` is called already by `Distribution.__init__`. This shows up when invoked from a directory with `pyproject.toml` or a similar config file. As the Build & Test workflow invokes the doctester from `SAGE_ROOT/src`, these errors show up in all tests. Marking it as a blocker so it is [applied automatically in CI workflows](https://github.com/sagemath/sage/wiki/Sage-10.2-Release- Tour#open-blocker-prs-are-applied-automatically-in-ci-workflows)  Fixes sagemath#36400 (comment)  ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [ ] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36552 Reported by: Matthias Köppe Reviewer(s): Tobias Diez

…agemath#36435   This repairs the installation method described in https://github.com/sagemath/sage#alternative-installation-using-pypi broken by changes in sagemath#36400, sagemath#36435. We also expand the tests for this distribution in its tox.ini. Run `(cd pkgs/sage-conf_pypi && tox -v -v -e py311)`    ### 📝 Checklist     - [x] The title is concise, informative, and self-explanatory. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies   URL: sagemath#36533 Reported by: Matthias Köppe Reviewer(s): Kwankyu Lee

mkoeppe added the c: packages: standard label Oct 4, 2023

mkoeppe self-assigned this Oct 4, 2023

build/pkgs/setuptools_scm: Update to 8.0.4, change to wheel package

8361771

mkoeppe force-pushed the setuptools_scm_8 branch from 0ff738d to 8361771 Compare October 4, 2023 17:21

Matthias Koeppe added 2 commits October 4, 2023 13:58

Add more workarounds for setuptools_scm + --no-build-isolation

f65ae6b

src/sage/misc/cython.py: Suppress a warning caused by setuptools_scm 8

0638968

mkoeppe changed the title ~~build/pkgs/setuptools_scm: Update to 8.0.4~~ build/pkgs/setuptools_scm: Update to 8.0.4, add fixes for version 8 Oct 4, 2023

mkoeppe requested a review from kiwifb October 4, 2023 21:01

pkgs/sage-*: Add setuptools_scm workaround to setup.py

114a610

mkoeppe added the s: needs review label Oct 5, 2023

mkoeppe requested a review from tobiasdiez October 5, 2023 01:37

tobiasdiez reviewed Oct 5, 2023

View reviewed changes

Merge branch 'develop' into setuptools_scm_8

2dad61b

mkoeppe requested a review from dimpase October 8, 2023 16:01

mkoeppe added the p: critical / 2 label Oct 10, 2023

dimpase approved these changes Oct 16, 2023

View reviewed changes

dimpase added s: positive review and removed s: needs review labels Oct 16, 2023

vbraun merged commit e73e211 into sagemath:develop Oct 21, 2023
18 of 52 checks passed

github-actions bot removed the s: positive review label Oct 21, 2023

mkoeppe added this to the sage-10.2 milestone Oct 21, 2023

mkoeppe deleted the setuptools_scm_8 branch October 22, 2023 06:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`build/pkgs/setuptools_scm`: Update to 8.0.4, add fixes for version 8 #36400

`build/pkgs/setuptools_scm`: Update to 8.0.4, add fixes for version 8 #36400

mkoeppe commented Oct 4, 2023 •

edited

Loading

kiwifb commented Oct 4, 2023

mkoeppe commented Oct 4, 2023

kiwifb commented Oct 4, 2023

github-actions bot commented Oct 5, 2023

tobiasdiez left a comment

mkoeppe commented Oct 5, 2023

dimpase commented Oct 16, 2023

dimpase left a comment

tobiasdiez commented Oct 16, 2023

dimpase commented Oct 16, 2023

tobiasdiez commented Oct 16, 2023

mkoeppe commented Oct 17, 2023

kiwifb commented Oct 17, 2023

mkoeppe commented Oct 17, 2023

kiwifb commented Oct 17, 2023

tobiasdiez commented Oct 26, 2023

kiwifb commented Oct 26, 2023

mkoeppe commented Oct 26, 2023

mkoeppe commented Oct 26, 2023

build/pkgs/setuptools_scm: Update to 8.0.4, add fixes for version 8 #36400

build/pkgs/setuptools_scm: Update to 8.0.4, add fixes for version 8 #36400

Conversation

mkoeppe commented Oct 4, 2023 • edited Loading

📝 Checklist

⌛ Dependencies

kiwifb commented Oct 4, 2023

mkoeppe commented Oct 4, 2023

kiwifb commented Oct 4, 2023

github-actions bot commented Oct 5, 2023

tobiasdiez left a comment

Choose a reason for hiding this comment

mkoeppe commented Oct 5, 2023

dimpase commented Oct 16, 2023

dimpase left a comment

Choose a reason for hiding this comment

tobiasdiez commented Oct 16, 2023

dimpase commented Oct 16, 2023

tobiasdiez commented Oct 16, 2023

mkoeppe commented Oct 17, 2023

kiwifb commented Oct 17, 2023

mkoeppe commented Oct 17, 2023

kiwifb commented Oct 17, 2023

tobiasdiez commented Oct 26, 2023

kiwifb commented Oct 26, 2023

mkoeppe commented Oct 26, 2023

mkoeppe commented Oct 26, 2023

`build/pkgs/setuptools_scm`: Update to 8.0.4, add fixes for version 8 #36400

`build/pkgs/setuptools_scm`: Update to 8.0.4, add fixes for version 8 #36400

mkoeppe commented Oct 4, 2023 •

edited

Loading